Skip to content

fix: add input validation for CSV import (#554)#751

Open
AyeshaaRafaqat wants to merge 1 commit into
OWASP:mainfrom
AyeshaaRafaqat:fix/554-csv-input-validation
Open

fix: add input validation for CSV import (#554)#751
AyeshaaRafaqat wants to merge 1 commit into
OWASP:mainfrom
AyeshaaRafaqat:fix/554-csv-input-validation

Conversation

@AyeshaaRafaqat

@AyeshaaRafaqat AyeshaaRafaqat commented Feb 21, 2026

Copy link
Copy Markdown

Fixes #554

  • Validates CRE column format matches XXX-XXX| pattern
  • Returns descriptive 400 error instead of generic 500
  • Handles triple-quoted text edge case

Tested locally by reproducing both cases from the issue.

@AyeshaaRafaqat

AyeshaaRafaqat commented Feb 21, 2026

Copy link
Copy Markdown
Author

@northdpole would appreciate any feedback on the approach here happy to make changes based on your review.

@PRAteek-singHWY

PRAteek-singHWY commented Feb 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hi @AyeshaaRafaqat , thanks for working on this!

Just a heads up , this issue is also being addressed in PR #682 and PR #683 , which is currently under maintainer review. I hadn’t linked it to the issue earlier while working on it, because it was part of while I was working on MyOpenCRE milestone and was suggested by @northdpole to move my logic into "spreadsheet_parsers.py" with a new function
" def validate_import_csv_rows "

U can refer to this for more information -> #584 (comment)

Flagging this here so maintainers are aware of the overlap and can decide the best path forward. Appreciate the effort on this!
Great work!

@Yashwin-i Yashwin-i mentioned this pull request Mar 4, 2026
Open
@northdpole @AyeshaaRafaqat @cursoragent
fix: rewrite CSV import validation for MyOpenCRE path (OWASP#554)
f8a5273 
Rebase OWASP#751 onto current main: UTF-8/triple-quote/csv.Error checks in
web_main, CRE XXX-XXX|Name validation in myopencre_parser, plus tests.

Co-authored-by: Ayesha Rafaqat <ayesharafaqat14@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@northdpole northdpole force-pushed the fix/554-csv-input-validation branch from ef25746 to f8a5273 Compare July 13, 2026 17:19
northdpole
northdpole approved these changes Jul 13, 2026
View reviewed changes

@northdpole northdpole left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @AyeshaaRafaqat for the original #554 CSV validation work.

This updates #751 by rebasing onto current main and rewriting the checks onto the live MyOpenCRE import path (myopencre_parser / cre_csv_import). The legacy spreadsheet_parsers path from the earlier PR no longer applies after the import pipeline move.

What landed:

  • web_main: UTF-8 decode errors, triple-quote rejection (bogus root CREs), and csv.Error → HTTP 400
  • myopencre_parser.validate_cre_csv_rows: require \d{3}-\d{3}|Name for non-empty CRE cells
  • Tests covering parser validation plus the CSV import endpoint

Targeted tests for the new cases passed locally. CI should go green on this head — CODEOWNERS, please take a look if needed.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: b3d10946-27a8-4daf-a4f2-c9249300e092

📥 Commits

Reviewing files that changed from the base of the PR and between 6b6a5ac and f8a5273.

📒 Files selected for processing (4)
  • application/tests/myopencre_parser_test.py
  • application/tests/web_main_test.py
  • application/utils/external_project_parsers/parsers/myopencre_parser.py
  • application/web/web_main.py

Summary by CodeRabbit

  • Bug Fixes
    • Improved CRE CSV import validation for malformed CRE identifiers and missing separators.
    • Added clear error messages with row and column context for invalid CRE values.
    • Rejected unsupported triple-quoted CSV content.
    • Added a specific error for CSV files that cannot be decoded as UTF-8.
    • Empty CRE cells are now handled correctly during import.

Walkthrough

Changes

CRE CSV validation

Layer / File(s) Summary
CRE cell validation and unit coverage
application/utils/external_project_parsers/parsers/myopencre_parser.py, application/tests/myopencre_parser_test.py		 CRE-prefixed cells are validated against XXX-XXX|Name; empty values are skipped and invalid values raise contextual ValueErrors.
Parser validation integration
application/utils/external_project_parsers/parsers/myopencre_parser.py		 parse_rows_to_documents validates rows before shared export-format parsing.
HTTP import guards and endpoint coverage
application/web/web_main.py, application/tests/web_main_test.py		 The import endpoint handles invalid UTF-8, rejects triple-quoted content, parses materialized rows, and tests malformed import responses.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: robvanderveer, northdpole, paoga87, pa04rth

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main change: adding input validation for CSV import.
Description check ✅ Passed The description matches the PR by describing CRE format validation, 400 errors, and triple-quoted edge cases.
Linked Issues check ✅ Passed The changes address issue #554 by rejecting invalid CRE formats and triple-quoted rows instead of importing bad data.
Out of Scope Changes check ✅ Passed The modified files and tests all support CSV import validation, with no clear unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@northdpole northdpole northdpole approved these changes
@robvanderveer robvanderveer Awaiting requested review from robvanderveer robvanderveer is a code owner
@Pa04rth Pa04rth Awaiting requested review from Pa04rth Pa04rth is a code owner
@paoga87 paoga87 Awaiting requested review from paoga87 paoga87 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Input validation missing on import csv functionality

3 participants

@AyeshaaRafaqat @PRAteek-singHWY @northdpole